Repository navigation
Make the output filename flexible and change default to ISO 8601 datetime - #30
Merged
KristianHMoller merged 8 commits intoNov 13, 2026
Conversation
KristianHMoller
changed the title
KristianHMoller
marked this pull request as ready for review
November 12, 2026 08:55
elenibriola
reviewed
Nov 13, 2026
elenibriola
left a comment
elenibriola
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the changes, Kristian! The implementation looks good overall and I like the additional flexibility for configuring filenames through environment variables.
I have a few comments/questions:
- Would it make sense to include the dataset name in the default output filename to avoid potential collisions when different datasets are processed for the same timestamp and stored in the same location?
- I will add a few unit tests for the filename generation and configuration precedence. I will start the pr for it these days. I think it is time.
Otherwise the changes look good to me!
Collaborator
Author
|
Excellent idea with the dataset in the default output filename. |
elenibriola
self-requested a review
November 13, 2026 09:59
elenibriola
approved these changes
Nov 13, 2026
elenibriola
left a comment
elenibriola
left a comment
Collaborator
There was a problem hiding this comment.
I have nothing else to add. Looks good to me! Nice work!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds the option of providing a custom output filename format as opposed to this being hardcoded. The custom output filename format can be specified for a given dataset in
config.yamlas currently done forKNMI_MTG.Additionally, the default output filename format has been changed to have an ISO 8601-compliant datetime format.
Default output filename format:
SolarNowcast_YYYY-MM-DDTHHMMZ.ncSelected output filename format for KNMI_MTG:
SolarNowcast_MTG-CPP_YYYY-MM-DDTHHMMZ.ncFor legacy reasons, the default remains unchanged with KNMI_MSG:
SolarNowcast_YYYYMMDDHHMM.ncWith this PR, the
filename_formatpreviously used in the config.yaml for the input filename format has been changed toinput_filename_format, so runs with oldconfig.yamlfiles will fail.Additional flexibility has been added to allow the
INPUT_FILENAME_FORMATandOUTPUT_FILENAME_FORMATenvironment variables to overwrite the corresponding filename formats so that the formats can be set without changing the code.Finally, the
datasetvariable has been added to the metadata of the output NetCDF file with the labelsatellite_data_sourcefor transparency.